-
Notifications
You must be signed in to change notification settings - Fork 841
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add EuiColorPalettePicker #3192
Conversation
Preview documentation changes for this PR: https://eui.elastic.co/pr_3192/ |
Preview documentation changes for this PR: https://eui.elastic.co/pr_3192/ |
Preview documentation changes for this PR: https://eui.elastic.co/pr_3192/ |
Preview documentation changes for this PR: https://eui.elastic.co/pr_3192/ |
Preview documentation changes for this PR: https://eui.elastic.co/pr_3192/ |
Update some types
Preview documentation changes for this PR: https://eui.elastic.co/pr_3192/ |
Preview documentation changes for this PR: https://eui.elastic.co/pr_3192/ |
Preview documentation changes for this PR: https://eui.elastic.co/pr_3192/ |
1 similar comment
Preview documentation changes for this PR: https://eui.elastic.co/pr_3192/ |
203a517
to
c3ae30b
Compare
Preview documentation changes for this PR: https://eui.elastic.co/pr_3192/ |
…_picker.tsx Co-authored-by: Greg Thompson <[email protected]>
…_picker.tsx Co-authored-by: Greg Thompson <[email protected]>
…into color-palette-picker
Co-authored-by: Greg Thompson <[email protected]>
…_picker.tsx Co-authored-by: Greg Thompson <[email protected]>
Preview documentation changes for this PR: https://eui.elastic.co/pr_3192/ |
Preview documentation changes for this PR: https://eui.elastic.co/pr_3192/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉 LGTM! We need to get this into Kibana ASAP 😆
It looks like there's a still a couple unchecked items in the PR checklist though that you might want to cover.
Preview documentation changes for this PR: https://eui.elastic.co/pr_3192/ |
Thanks @cchaos, I'm covering now the unchecked items from the PR checklist and I'll be waiting for another review from @thompsongl because I changed a few TS things. |
jenkins test this |
Preview documentation changes for this PR: https://eui.elastic.co/pr_3192/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're close here. I just want to solidify the EuiColorPalettePickerPaletteProps
interface so it's a bit more clear to consumers how to display what they want.
src/components/color_picker/color_palette_picker/color_palette_picker.tsx
Outdated
Show resolved
Hide resolved
PR: elizabetdev#5 |
More strict types
Preview documentation changes for this PR: https://eui.elastic.co/pr_3192/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM. @miukimiu will have some follow-up work on the palette display bit, which may also enable more flexibility with option types (fixed
palettes with ColorStop
values, for instance). For now, we're protected against error states and malformed options.
* Adding initial code * Adding initial code * Adding getLinearGradient method * Improving palettes * initial tests * More improvements * Deleting palettes file * Adding display togles * Simplifying button * Custom option value * Update some types * getFixedLinearGradient * Text as an option * Improving props description * Improving sass * Adding selectionDisplay * More examples * Addressing pr review * Update src-docs/src/views/color_picker/color_palette_picker.tsx Co-authored-by: Caroline Horn <[email protected]> * Update src/components/color_picker/color_palette_picker/color_palette_picker.tsx Co-authored-by: Caroline Horn <[email protected]> * Update src/components/color_picker/color_palette_picker/color_palette_picker.tsx Co-authored-by: Caroline Horn <[email protected]> * Extending EuiSuperSelectProp * Simplifying structure * Changelog * Improving example * Improving snippet * Adding more tests * Improving props description * Improving docs text * Improve text * Addressing review * Update src/components/color_picker/utils.ts Co-authored-by: Greg Thompson <[email protected]> * Update src/components/color_picker/utils.ts Co-authored-by: Greg Thompson <[email protected]> * Update src/components/color_picker/color_palette_picker/color_palette_picker.tsx Co-authored-by: Greg Thompson <[email protected]> * Update src/components/color_picker/color_palette_picker/color_palette_picker.tsx Co-authored-by: Greg Thompson <[email protected]> * Update src/components/color_picker/utils.ts Co-authored-by: Greg Thompson <[email protected]> * Update src/components/color_picker/color_palette_picker/color_palette_picker.tsx Co-authored-by: Greg Thompson <[email protected]> * Improving TS * Improving palette doc * Palette doc * Changing name of gray palette * Slimming the description a bit * removing console log * more strict types * Update src/components/color_picker/color_palette_picker/color_palette_picker.tsx Co-authored-by: cchaos <[email protected]> Co-authored-by: Caroline Horn <[email protected]> Co-authored-by: Greg Thompson <[email protected]>
Summary
Closes #2795
This PR adds a Color Palette Picker component called EuiColorPalettePicker.
While working on this PR I tried to address a few suggestions:
gradient
should be an array of objects[{stop: 100, color: '#54B399'}]
rather than a CSSlinear-gradient
fixed
palette should be a gradient rather than different flex items with a background. The reason for changing this is that the flex items create a small empty space on the right side. Also thebox-shadow: inset
doesn't work with the flex items.EuiSuperSelect
. I tried and the code became complex and for this reason, I created atext
option. I think it is a simpler solution and also we don't lose the purpose of the palette pickerChecklist
[ ] Checked for breaking changes and labeled appropriately